@zdexter Zach, this one looks good, apart from the missing curly brackets. Do you think you could amend the commit?

Also, did you sign the CLA already?

For individuals (a simple click-through form):
http://code.google.com/legal/individual-cla-v1.0.html

For corporations (print, sign and scan+email, fax or mail):
http://code.google.com/legal/corporate-cla-v1.0.html"

@pkozlowski-opensource Thanks for pointing that out; I've amended the commit, run tests, and pushed. Also, yes, I did sign the CLA back when I opened the pull request.

Good question about checking for undefined vs a falsy value when determining the presence of an optional argument. I looked into it and couldn't find a good programmatic reason to choose one over the other. But there is one edge case - it seems falsy values should be invalid, but actually, this is not true with iFrame names in HTML. Someone could conceivably have a frame name that evaluates to a falsy value. I tested this with an iFrame where name="0", and sure enough, Chrome considered "0" considered falsy even though it was a valid frame name. So I left in the check for undefined.

@zdexter Thnxk for taking care of this one and signing CLA. For the undefined testing you could eventually use angular's isUndefined.

@pkozlowski-opensource isDefined is only available in linky.js via "angular.isDefined". Is it okay to reference the function that way? If so, I can amend the commit with the tested update.

angular.isDefined is an appropriate public API. ngResource module uses it
this way:
https://github.com/angular/angular.js/blob/master/src/ngResource/resource.js#L289

On 18 November 2012 16:15, Zach Dexter notifications@github.com wrote:

@pkozlowski-opensource https://github.com/pkozlowski-opensourceisDefined is only available in linky.js via "angular.isDefined". Is it okay
to reference the function that way? If so, I can amend the commit with the
tested update.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1446#issuecomment-10487715.

Thanks. Amended commit to use angular.isDefined(), unit and scenario tested, and pushed.

otherwise this looks good

I made some changes and landed this as 610927d

thanks a lot! if you don't have our t-shirt yet please fill out this form: http://goo.gl/075Sj